Skip to content

Remove sha1 from default features in gix-hash#2441

Merged
Sebastian Thiel (Byron) merged 3 commits intoGitoxideLabs:mainfrom
cruessler:remove-sha-1-from-default-features
Feb 27, 2026
Merged

Remove sha1 from default features in gix-hash#2441
Sebastian Thiel (Byron) merged 3 commits intoGitoxideLabs:mainfrom
cruessler:remove-sha-1-from-default-features

Conversation

@cruessler
Copy link
Copy Markdown
Contributor

@cruessler Christoph Rüßler (cruessler) commented Feb 21, 2026

  • feat!: remove sha1 from default features in gix-hash

  • Adapt to sha1 not being default feature of gix-hash

  • Make gix depend on gix-hash with sha1 enabled

This PR removes sha1 from gix-hash’s default features. It adds a feature sha1 to all depending crates that did not compile anymore after the removal. Now, just check passes and I want to get feedback from CI to see where else I need to adapt to these changes.

This PR also adds sha1 to features in the [package.metadata.docs.rs] section of all crates that now have a feature flag sha1.

This PR is part of #281.

@Byron Sebastian Thiel (Byron) force-pushed the remove-sha-1-from-default-features branch from 15f0e75 to f6fa94b Compare February 23, 2026 05:44
@Byron Sebastian Thiel (Byron) force-pushed the remove-sha-1-from-default-features branch from f6fa94b to 37d0acc Compare February 23, 2026 05:47
@Byron
Copy link
Copy Markdown
Member

Thanks a lot for keeping the sha256-ball rolling!

Actually this is really nice as it reveals crates with a direct relationship to gix-hash, which then can probably be a focus for testing as well.

I will go back into hiding until the PR comes out of draft.

@cruessler
Copy link
Copy Markdown
Contributor Author

Quick status update: I believe I’ve now made all the follow-up changes necessary to make CI green. The last test failure seems not related to my changes as far as I can tell. I plan on updating the description soon, giving a higher-level overview of all the changes and some observations I made along the way.

@EliahKagan
Copy link
Copy Markdown
Member

Quick status update: I believe I’ve now made all the follow-up changes necessary to make CI green.

That seems likely--I just reran the one failed job and it succeeded.

@Byron Sebastian Thiel (Byron) marked this pull request as ready for review February 26, 2026 12:44
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6177174fdf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread gix/Cargo.toml
@Byron
Copy link
Copy Markdown
Member

Review Notes

  • gix also must make the default hashes configurable, as users should be able to turn it off by not enabling default features.
  • in order to build crate documentation successfully, all crates now have to forward sha1 effectively to configure docs.rs to set it.

@Byron Sebastian Thiel (Byron) force-pushed the remove-sha-1-from-default-features branch 6 times, most recently from 13a1efe to 63e3e6a Compare February 27, 2026 05:30
This way one can control which hashes are compiled in exactly,
while having reasonable defaults automatically.
@Byron Sebastian Thiel (Byron) force-pushed the remove-sha-1-from-default-features branch from 63e3e6a to 3832916 Compare February 27, 2026 06:26
@Byron Sebastian Thiel (Byron) merged commit e8bf096 into GitoxideLabs:main Feb 27, 2026
45 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants